Skip to content

Fix initialization issue with single smooth tendency#18

Merged
SBlokhuizen merged 3 commits intodevelopfrom
bugfix/fix-single-smooth
May 26, 2025
Merged

Fix initialization issue with single smooth tendency#18
SBlokhuizen merged 3 commits intodevelopfrom
bugfix/fix-single-smooth

Conversation

@SBlokhuizen
Copy link
Copy Markdown
Collaborator

@SBlokhuizen SBlokhuizen commented Feb 18, 2025

This fixes a bug where the spline object of the smooth tendency was not properly updated after creation.

For example:

waveform:
- {type: smooth, from: 0, to: 1, duration: 10}

Before:
image

After:
image

@SBlokhuizen SBlokhuizen self-assigned this Feb 18, 2025
self.from_ = 0.0
self.to = 0.0
super().__init__(**kwargs)
self._update_values()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_update_values also has the on_init=True flag in the @depends decorator. Do you know why that is not sufficient?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the on_init=True will cause the method to be called when the __init__ of the param.Parametrized base class is called, namely here. Since we now set the parameters after the intialization, the method is not called again.

Alternatively, a more declarative way to handle this would be:

@depends(
    "_trigger",
    "from_",
    "to",
    "start",
    "end",
    "start_derivative",
    "end_derivative",
    watch=True,
    on_init=True,
)

At the expense of more triggers. We then should also account for the intermediate triggers (for example, CubicSpline will throw an error if start > end, which could occur if start is updated before end).

@SBlokhuizen SBlokhuizen merged commit ae636c5 into develop May 26, 2025
4 checks passed
@SBlokhuizen SBlokhuizen deleted the bugfix/fix-single-smooth branch May 26, 2025 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants